-
Notifications
You must be signed in to change notification settings - Fork 125
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[MHV-55186] Update all V1 components to V3 web components on SM MHV #28504
Conversation
- Va-Button - Va-Alert - Va-Accordion / Va-Accordion-Item
- Recipient dropdown - CrisisLineConnectionButton - Connect with crisis line button - Edit signature modal - Route leaving guard modal
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Awesome 😃 Just one minor comment to get clarification on heading
className="vads-u-margin-bottom--0 vads-u-font-size--base vads-font-family-sans-serif" | ||
> | ||
File attached | ||
</h3> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Was this h3
suggested by a11y? trying to see the rationale behind replacing p
with heading tag
…s://github.com/department-of-veterans-affairs/vets-website into 55186-Update-all-V1-components-to-V6-on-sm-mhv
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
With the merge of this PR on Friday, you should no longer need to include the uswds
prop in any of your web component declarations.
@vsaleem per #28504 (review) as we chatted on Friday, at this point we would only need |
…s://github.com/department-of-veterans-affairs/vets-website into 55186-Update-all-V1-components-to-V6-on-sm-mhv
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Icon found
Icons can be decorative, but sometimes they are used to convey meaning. If there are any semantics associated with an icon, those semantics should also be conveyed to a screen reader.
What you can do
Review the markup and see if the icon provides information that isn't represented textually, or wait for a VSP review.
))} | ||
{activeFolder.folderId !== DefaultFolders.DRAFTS.id && unreadMessages && ( | ||
<span> | ||
<i |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
icon found
))} | ||
{activeFolder.folderId !== DefaultFolders.DRAFTS.id && unreadMessages && ( | ||
<span> | ||
<i |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
icon found
Closing this pull request as it is no longer needed. These issues are being handled in individual stories. V3 Component-Issues resulting from upgrade |
Summary
V1 Web-Components are retired. This update switches all va-components to utilize the V3 uswds web-components.
Related issue(s)
[MHV-55186] Update Retired V1 components to V3
Testing done
Screenshots
Note: This field is mandatory for UI changes (non-component work should NOT have screenshots).
What areas of the site does it impact?
(Va.gov - Secure Messaging)
Acceptance criteria
AC1 All components have been upgraded to V3
AC2 V3 Components to test:
Quality Assurance & Testing
Error Handling
Authentication
#sitewide-public-websites
Slack channel for questionsRequested Feedback
(OPTIONAL) What should the reviewers know in addition to the above. Is there anything specific you wish the reviewer to assist with. Do you have any concerns with this PR, why?